From 06589a86db24ec5e0d2b21783f577c714105a7bf Mon Sep 17 00:00:00 2001 From: Noah Fontes Date: Mon, 31 Aug 2015 16:33:29 -0700 Subject: [PATCH] Fix platform-specific dependency resolution for custom builds. This change resolves dependencies against all possible platforms. Activation of dependencies for a given target in the context of one or more platforms is now wholly handled by cargo_rustc. --- src/cargo/core/dependency.rs | 9 -- src/cargo/core/resolver/mod.rs | 29 +--- src/cargo/ops/cargo_compile.rs | 3 - src/cargo/ops/cargo_rustc/context.rs | 38 ++--- src/cargo/ops/cargo_rustc/custom_build.rs | 7 +- src/cargo/ops/cargo_rustc/fingerprint.rs | 2 +- src/cargo/ops/cargo_rustc/mod.rs | 17 ++- tests/test_cargo_cross_compile.rs | 168 ++++++++++++++++++++++ 8 files changed, 207 insertions(+), 66 deletions(-) diff --git a/src/cargo/core/dependency.rs b/src/cargo/core/dependency.rs index bf7378fd5..651d5f3d0 100644 --- a/src/cargo/core/dependency.rs +++ b/src/cargo/core/dependency.rs @@ -152,15 +152,6 @@ impl Dependency { (self.only_match_name || (self.req.matches(id.version()) && &self.source_id == id.source_id())) } - - /// Returns true if the dependency should be built for this platform. - pub fn is_active_for_platform(&self, platform: &str) -> bool { - match self.only_for_platform { - None => true, - Some(ref p) if *p == platform => true, - _ => false - } - } } #[derive(PartialEq,Clone,RustcEncodable)] diff --git a/src/cargo/core/resolver/mod.rs b/src/cargo/core/resolver/mod.rs index aa5179219..702248260 100644 --- a/src/cargo/core/resolver/mod.rs +++ b/src/cargo/core/resolver/mod.rs @@ -138,7 +138,6 @@ pub enum Method<'a> { dev_deps: bool, features: &'a [String], uses_default_features: bool, - target_platform: Option<&'a str>, }, } @@ -292,14 +291,7 @@ fn activate(mut cx: Box, let deps = try!(cx.build_deps(registry, parent, method)); - // Extracting the platform request. - let platform = match *method { - Method::Required { target_platform, .. } => target_platform, - Method::Everything => None, - }; - - activate_deps(cx, registry, parent, platform, deps.iter(), 0, - &mut |mut cx, registry| { + activate_deps(cx, registry, parent, deps.iter(), 0, &mut |mut cx, registry| { cx.visited.remove(id); finished(cx, registry) }) @@ -312,16 +304,12 @@ fn activate(mut cx: Box, /// provided is an iterator over all dependencies where each element yielded /// informs us what the candidates are for the dependency in question. /// -/// The `platform` argument is the target platform that the dependencies are -/// being activated for. -/// /// If all dependencies can be activated and resolved to a version in the /// dependency graph the `finished` callback is invoked with the current state /// of the world. fn activate_deps<'a>(cx: Box, registry: &mut Registry, parent: &Summary, - platform: Option<&'a str>, mut deps: slice::Iter<'a, DepInfo>, cur: usize, finished: &mut FnMut(Box, &mut Registry) -> ResolveResult) @@ -335,7 +323,6 @@ fn activate_deps<'a>(cx: Box, dev_deps: false, features: features, uses_default_features: dep.uses_default_features(), - target_platform: platform, }; let prev_active = cx.prev_active(dep); @@ -384,8 +371,7 @@ fn activate_deps<'a>(cx: Box, } let my_cx = try!(activate(my_cx, registry, candidate, &method, &mut |cx, registry| { - activate_deps(cx, registry, parent, platform, deps.clone(), cur + 1, - finished) + activate_deps(cx, registry, parent, deps.clone(), cur + 1, finished) })); match my_cx { Ok(cx) => return Ok(Ok(cx)), @@ -680,17 +666,6 @@ impl Context { let deps = parent.dependencies(); let deps = deps.iter().filter(|d| d.is_transitive() || dev_deps); - // Second, ignoring dependencies that should not be compiled for this - // platform - let deps = deps.filter(|d| { - match *method { - Method::Required{target_platform: Some(ref platform), ..} => { - d.is_active_for_platform(platform) - }, - _ => true - } - }); - let (mut feature_deps, used_features) = try!(build_features(parent, method)); let mut ret = Vec::new(); diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index b23646ca2..74d0b0942 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -145,13 +145,10 @@ pub fn compile_pkg<'a>(package: &Package, try!(registry.add_overrides(override_ids)); - let platform = target.as_ref().unwrap_or(&config.rustc_info().host); - let method = Method::Required { dev_deps: true, // TODO: remove this option? features: &features, uses_default_features: !no_default_features, - target_platform: Some(&platform[..]), }; let resolved_with_overrides = diff --git a/src/cargo/ops/cargo_rustc/context.rs b/src/cargo/ops/cargo_rustc/context.rs index 9ad470060..d9565294d 100644 --- a/src/cargo/ops/cargo_rustc/context.rs +++ b/src/cargo/ops/cargo_rustc/context.rs @@ -162,7 +162,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> { } for &(target, profile) in targets { - self.build_requirements(pkg, target, profile, Platform::Target); + self.build_requirements(pkg, target, profile, Kind::from(target)); } let jobs = self.jobs(); @@ -177,8 +177,9 @@ impl<'a, 'cfg> Context<'a, 'cfg> { } fn build_requirements(&mut self, pkg: &'a Package, target: &'a Target, - profile: &Profile, req: Platform) { - let req = if target.for_host() {Platform::Plugin} else {req}; + profile: &Profile, kind: Kind) { + let req = if kind == Kind::Host { Platform::Plugin } else { Platform::Target }; + match self.requirements.entry((pkg.package_id(), target.name())) { Occupied(mut entry) => match (*entry.get(), req) { (Platform::Plugin, Platform::Plugin) | @@ -191,15 +192,14 @@ impl<'a, 'cfg> Context<'a, 'cfg> { Vacant(entry) => { entry.insert(req); } }; - for &(pkg, dep, profile) in self.dep_targets(pkg, target, profile).iter() { - self.build_requirements(pkg, dep, profile, req); + for (pkg, dep, profile) in self.dep_targets(pkg, target, kind, profile) { + self.build_requirements(pkg, dep, profile, kind.for_target(dep)); } match pkg.targets().iter().find(|t| t.is_custom_build()) { Some(custom_build) => { let profile = self.build_script_profile(pkg.package_id()); - self.build_requirements(pkg, custom_build, profile, - Platform::Plugin); + self.build_requirements(pkg, custom_build, profile, Kind::Host); } None => {} } @@ -339,7 +339,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> { /// For a package, return all targets which are registered as dependencies /// for that package. - pub fn dep_targets(&self, pkg: &Package, target: &Target, + pub fn dep_targets(&self, pkg: &Package, target: &Target, kind: Kind, profile: &Profile) -> Vec<(&'a Package, &'a Target, &'a Profile)> { if profile.doc { @@ -365,6 +365,18 @@ impl<'a, 'cfg> Context<'a, 'cfg> { target.is_example() || profile.test; + // If this dependency is only available for certain platforms, + // make sure we're only enabling it for that platform. + let is_platform_same = match (d.only_for_platform(), kind) { + (Some(ref platform), Kind::Host) => { + *platform == self.config.rustc_info().host + }, + (Some(ref platform), Kind::Target) => { + *platform == self.target_triple + }, + (None, _) => true + }; + // If the dependency is optional, then we're only activating it // if the corresponding feature was activated let activated = !d.is_optional() || @@ -372,7 +384,7 @@ impl<'a, 'cfg> Context<'a, 'cfg> { f.contains(d.name()) }).unwrap_or(false); - is_correct_dep && is_actual_dep && activated + is_correct_dep && is_actual_dep && is_platform_same && activated }) }).filter_map(|pkg| { pkg.targets().iter().find(|t| t.is_lib()).map(|t| { @@ -510,12 +522,4 @@ impl Platform { _ => false, } } - - pub fn each_kind(self, mut f: F) where F: FnMut(Kind) { - match self { - Platform::Target => f(Kind::Target), - Platform::Plugin => f(Kind::Host), - Platform::PluginAndTarget => { f(Kind::Target); f(Kind::Host); } - } - } } diff --git a/src/cargo/ops/cargo_rustc/custom_build.rs b/src/cargo/ops/cargo_rustc/custom_build.rs index 0c939dd59..a12f27bfd 100644 --- a/src/cargo/ops/cargo_rustc/custom_build.rs +++ b/src/cargo/ops/cargo_rustc/custom_build.rs @@ -91,7 +91,8 @@ pub fn prepare(pkg: &Package, target: &Target, req: Platform, let not_custom = pkg.targets().iter().find(|t| { !t.is_custom_build() }).unwrap(); - cx.dep_targets(pkg, not_custom, profile).iter().filter_map(|&(pkg, t, _)| { + cx.dep_targets(pkg, not_custom, kind, profile).iter() + .filter_map(|&(pkg, t, _)| { if !t.linkable() { return None } pkg.manifest().links().map(|links| { (links.to_string(), pkg.package_id().clone()) @@ -381,7 +382,7 @@ pub fn build_map<'b, 'cfg>(cx: &mut Context<'b, 'cfg>, // kind that we're compiling for, and otherwise just do a quick // pre-flight check to see if we've already calculated the set of // dependencies. - let kind = if target.for_host() {Kind::Host} else {kind}; + let kind = kind.for_target(target); let id = pkg.package_id(); if out.contains_key(&(id, target, profile, kind)) { return &out[&(id, target, profile, kind)] @@ -398,7 +399,7 @@ pub fn build_map<'b, 'cfg>(cx: &mut Context<'b, 'cfg>, // linkable to us (e.g. not a binary) and it's for the same original // `kind`. let mut ret = Vec::new(); - for &(pkg, target, p) in cx.dep_targets(pkg, target, profile).iter() { + for (pkg, target, p) in cx.dep_targets(pkg, target, kind, profile) { let req = cx.get_requirement(pkg, target); let dep_kind = if req.includes(kind) { diff --git a/src/cargo/ops/cargo_rustc/fingerprint.rs b/src/cargo/ops/cargo_rustc/fingerprint.rs index 05336eebf..4cfd0dcd7 100644 --- a/src/cargo/ops/cargo_rustc/fingerprint.rs +++ b/src/cargo/ops/cargo_rustc/fingerprint.rs @@ -176,7 +176,7 @@ fn calculate<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, // elsewhere. Also skip fingerprints of binaries because they don't actually // induce a recompile, they're just dependencies in the sense that they need // to be built. - let deps = try!(cx.dep_targets(pkg, target, profile).into_iter() + let deps = try!(cx.dep_targets(pkg, target, kind, profile).into_iter() .filter(|&(_, t, _)| !t.is_custom_build() && !t.is_bin()) .map(|(pkg, target, profile)| { let kind = match kind { diff --git a/src/cargo/ops/cargo_rustc/mod.rs b/src/cargo/ops/cargo_rustc/mod.rs index 25d9f2a02..c18756968 100644 --- a/src/cargo/ops/cargo_rustc/mod.rs +++ b/src/cargo/ops/cargo_rustc/mod.rs @@ -127,8 +127,8 @@ pub fn compile_targets<'a, 'cfg: 'a>(targets: &[(&'a Target, &'a Profile)], if !target.is_lib() { continue } // Include immediate lib deps as well - for dep in cx.dep_targets(pkg, target, profile).iter() { - let (pkg, target, profile) = *dep; + for dep in cx.dep_targets(pkg, target, kind, profile) { + let (pkg, target, profile) = dep; let pkgid = pkg.package_id(); if !target.is_lib() { continue } if profile.doc { continue } @@ -187,7 +187,9 @@ fn compile<'a, 'cfg>(targets: &[(&'a Target, &'a Profile)], try!(rustc(pkg, target, profile, cx, req)) }; - for (work, kind) in work.into_iter() { + let kinds = work.iter().map(|&(_, kind)| kind).collect::>(); + + for (work, kind) in work { let (freshness, dirty, fresh) = try!(fingerprint::prepare_target(cx, pkg, target, profile, kind)); @@ -210,12 +212,15 @@ fn compile<'a, 'cfg>(targets: &[(&'a Target, &'a Profile)], (false, false, _) => jobs.queue(pkg, Stage::Binaries), }; dst.push((Job::new(dirty, fresh), freshness)); + } drop(profiling_marker); // Be sure to compile all dependencies of this target as well. - for &(pkg, target, p) in cx.dep_targets(pkg, target, profile).iter() { - try!(compile(&[(target, p)], pkg, cx, jobs)); + for kind in kinds { + for (pkg, target, p) in cx.dep_targets(pkg, target, kind, profile) { + try!(compile(&[(target, p)], pkg, cx, jobs)); + } } // If this is a custom build command, we need to not only build the @@ -710,7 +715,7 @@ fn build_deps_args(cmd: &mut CommandPrototype, cmd.env("OUT_DIR", &layout.build_out(package)); } - for &(pkg, target, p) in cx.dep_targets(package, target, profile).iter() { + for (pkg, target, p) in cx.dep_targets(package, target, kind, profile) { if target.linkable() { try!(link_to(cmd, pkg, target, p, cx, kind)); } diff --git a/tests/test_cargo_cross_compile.rs b/tests/test_cargo_cross_compile.rs index 489769e86..a7c42b216 100644 --- a/tests/test_cargo_cross_compile.rs +++ b/tests/test_cargo_cross_compile.rs @@ -696,3 +696,171 @@ test!(plugin_build_script_right_arch { {running} `rustc src[..]lib.rs [..]` ", compiling = COMPILING, running = RUNNING))); }); + +test!(build_script_with_platform_specific_dependencies { + if disabled() { return } + + let target = alternate(); + let host = ::rustc_host(); + let p = project("foo") + .file("Cargo.toml", r#" + [package] + name = "foo" + version = "0.0.1" + authors = [] + build = "build.rs" + + [build-dependencies.d1] + path = "d1" + "#) + .file("build.rs", "extern crate d1; fn main() {}") + .file("src/lib.rs", "") + .file("d1/Cargo.toml", &format!(r#" + [package] + name = "d1" + version = "0.0.0" + authors = [] + + [target.{}.dependencies] + d2 = {{ path = "../d2" }} + "#, host)) + .file("d1/src/lib.rs", "extern crate d2;") + .file("d2/Cargo.toml", r#" + [package] + name = "d2" + version = "0.0.0" + authors = [] + "#) + .file("d2/src/lib.rs", ""); + + assert_that(p.cargo_process("build").arg("-v").arg("--target").arg(&target), + execs().with_status(0) + .with_stdout(&format!("\ +{compiling} d2 v0.0.0 ([..]) +{running} `rustc d2[..]src[..]lib.rs [..]` +{compiling} d1 v0.0.0 ([..]) +{running} `rustc d1[..]src[..]lib.rs [..]` +{compiling} foo v0.0.1 ([..]) +{running} `rustc build.rs [..]` +{running} `{dir}[..]target[..]build[..]foo-[..]build-script-build` +{running} `rustc src[..]lib.rs [..] --target {target} [..]` +", compiling = COMPILING, running = RUNNING, dir = p.root().display(), target = target))); +}); + +test!(platform_specific_dependencies_do_not_leak { + if disabled() { return } + + let target = alternate(); + let host = ::rustc_host(); + let p = project("foo") + .file("Cargo.toml", r#" + [package] + name = "foo" + version = "0.0.1" + authors = [] + build = "build.rs" + + [dependencies.d1] + path = "d1" + + [build-dependencies.d1] + path = "d1" + "#) + .file("build.rs", "extern crate d1; fn main() {}") + .file("src/lib.rs", "") + .file("d1/Cargo.toml", &format!(r#" + [package] + name = "d1" + version = "0.0.0" + authors = [] + + [target.{}.dependencies] + d2 = {{ path = "../d2" }} + "#, host)) + .file("d1/src/lib.rs", "extern crate d2;") + .file("d2/Cargo.toml", r#" + [package] + name = "d2" + version = "0.0.0" + authors = [] + "#) + .file("d2/src/lib.rs", ""); + + assert_that(p.cargo_process("build").arg("-v").arg("--target").arg(&target), + execs().with_status(101) + .with_stderr("\ +[..] error: can't find crate for `d2` +[..] extern crate d2; +[..] +error: aborting due to previous error +Could not compile `d1`. + +Caused by: + [..] +")); +}); + +test!(platform_specific_variables_reflected_in_build_scripts { + if disabled() { return } + + let target = alternate(); + let host = ::rustc_host(); + let p = project("foo") + .file("Cargo.toml", &format!(r#" + [package] + name = "foo" + version = "0.0.1" + authors = [] + build = "build.rs" + + [target.{host}.dependencies] + d1 = {{ path = "d1" }} + + [target.{target}.dependencies] + d2 = {{ path = "d2" }} + "#, host = host, target = target)) + .file("build.rs", &format!(r#" + use std::env; + + fn main() {{ + let platform = env::var("TARGET").unwrap(); + let (expected, not_expected) = match &platform[..] {{ + "{host}" => ("DEP_D1_VAL", "DEP_D2_VAL"), + "{target}" => ("DEP_D2_VAL", "DEP_D1_VAL"), + _ => panic!("unknown platform") + }}; + + env::var(expected).unwrap(); + env::var(not_expected).unwrap_err(); + }} + "#, host = host, target = target)) + .file("src/lib.rs", "") + .file("d1/Cargo.toml", r#" + [package] + name = "d1" + version = "0.0.0" + authors = [] + links = "d1" + build = "build.rs" + "#) + .file("d1/build.rs", r#" + fn main() { println!("cargo:val=1") } + "#) + .file("d1/src/lib.rs", "") + .file("d2/Cargo.toml", r#" + [package] + name = "d2" + version = "0.0.0" + authors = [] + links = "d2" + build = "build.rs" + "#) + .file("d2/build.rs", r#" + fn main() { println!("cargo:val=1") } + "#) + .file("d2/src/lib.rs", ""); + + assert_that(p.cargo_process("build").arg("-v"), execs().with_status(0)); + assert_that(p.cargo_process("build").arg("-v").arg("--target").arg(&target), + execs().with_status(0)); +}); \ No newline at end of file -- 2.30.2